Conversation
📗 Scan Summary
|
| } | ||
|
|
||
| // Skip macOS resource fork files (._package.json) | ||
| baseName := filepath.Base(hdr.Name) |
There was a problem hiding this comment.
🎯 Static Application Security Testing (SAST) Vulnerability
| Severity | Finding |
|---|---|
![]() Low |
Untrusted stored input used in file paths, allowing access to unintended files. |
Full description
Vulnerability Details
| Rule ID: | go-stored-path-traversal |
Overview
Stored Path Traversal is a type of vulnerability that arises when user-controlled
input, such as file names or paths, is stored by the application and later used
without proper validation or sanitization to perform file operations. This can
allow an attacker to traverse directories and access or overwrite sensitive files
on the filesystem, potentially compromising the security and integrity of the
application or system.
Vulnerable example
func serveFile(w http.ResponseWriter, r *http.Request) {
row := db.QueryRow("SELECT file_path FROM files WHERE id = 12")
row.Scan(&filePath)
http.ServeFile(w, r, filePath)
}In this example, the serveFile function serves a file based on the file query
parameter provided by the user. However, in a real-world scenario, the filePath
variable might be retrieved from a stored source, such as a database or configuration
file, instead of being directly obtained from the request URL. The vulnerability
arises if the stored filePath is not properly validated or sanitized before being
used to serve files. Attackers could manipulate the stored filePath to perform
directory traversal attacks, potentially accessing sensitive files outside the
intended directory structure.
Remediation
To mitigate stored path traversal vulnerabilities, it is essential to validate
and sanitize user-controlled input before using it to construct file paths or
perform file operations. In this example, we can validate the file name to ensure
it does not contain directory traversal sequences before serving the file.
func serveFile(w http.ResponseWriter, r *http.Request) {
row := db.QueryRow("SELECT file_path FROM files WHERE id = ?", r.URL.Query().Get("id"))
row.Scan(&filePath)
+ // Validate file path to prevent directory traversal
+ if strings.Contains(filePath, "..") {
+ http.Error(w, "Invalid file path", http.StatusBadRequest)
+ return
+ }
http.ServeFile(w, r, filePath)
}Code Flows
Vulnerable data flow analysis result
tarReader.Next() (at artifactory/commands/pnpm/publish.go line 362)
hdr (at artifactory/commands/pnpm/publish.go line 362)
hdr (at artifactory/commands/pnpm/publish.go line 376)
hdr.Name (at artifactory/commands/pnpm/publish.go line 376)
filepath.Base(hdr.Name) (at artifactory/commands/pnpm/publish.go line 376)
There was a problem hiding this comment.
This code reuses the same logic from the existing NPM implementation (artifactory/commands/npm/publish.go#L370-L374), which also reads hdr.Name from tar headers for in-memory package.json filtering. The tar header name is never used to perform any file system operations - it's only used for in-memory filtering, and content is read via io.ReadAll(tarReader).
If this needs to be addressed, it should be done in a separate PR for both NPM and PNPM together.
| func extractRepoName(configUrl string) (string, error) { | ||
| url := strings.TrimSpace(configUrl) | ||
| url = strings.TrimPrefix(url, "https://") | ||
| url = strings.TrimPrefix(url, "http://") |
There was a problem hiding this comment.
🎯 Static Application Security Testing (SAST) Vulnerability
Full description
Vulnerability Details
| Rule ID: | go-insecure-protocol |
Overview
Using insecure protocols—such as HTTP, FTP, or LDAP—can expose sensitive
data during transmission, making it vulnerable to eavesdropping and man-in-the-middle
attacks. Secure protocols like HTTPS and FTPS should be used to ensure data
encryption during communication.
Vulnerable example
In this example, the application uses insecure protocols to communicate,
taking the protocol type from hardcoded strings.
package main
import (
"fmt"
)
type SwampService struct {
InsecureHttpProtocol string
InsecureFtpProtocol string
}
func NewSwampService() *SwampService {
return &SwampService{
InsecureHttpProtocol: "http://", // Insecure protocol
InsecureFtpProtocol: "ftp://", // Insecure protocol
}
}
func (s *SwampService) ConnectToFrogService(server string) {
url := s.InsecureHttpProtocol + server + "/frogEndpoint"
s.connect(url)
url = s.InsecureFtpProtocol + server + "/frogFile"
s.connect(url)
}
func (s *SwampService) connect(url string) {
fmt.Printf("Connecting to %s\n", url)
// Logic to connect to the service
}
func main() {
service := NewSwampService()
service.ConnectToFrogService("example.com")
}In this vulnerable example, the ConnectToFrogService method uses hardcoded
insecure protocols (HTTP and FTP) to connect, making communications susceptible
to attacks.
Remediation
To mitigate the use of insecure protocols, replace them with secure alternatives
such as HTTPS or FTPS.
package main
import (
"fmt"
)
type SwampService struct {
InsecureHttpProtocol string
InsecureFtpProtocol string
}
func NewSwampService() *SwampService {
return &SwampService{
InsecureHttpProtocol: "https://", // Insecure protocol
InsecureFtpProtocol: "ftps://", // Insecure protocol
}
}
func (s *SwampService) ConnectToFrogService(server string) {
url := s.InsecureHttpProtocol + server + "/frogEndpoint"
s.connect(url)
url = s.InsecureFtpProtocol + server + "/frogFile"
s.connect(url)
}
func (s *SwampService) connect(url string) {
fmt.Printf("Connecting to %s\n", url)
// Logic to connect to the service
}
func main() {
service := NewSwampService()
service.ConnectToFrogService("example.com")
}In this remediated example, the ConnectToFrogService method utilizes
secure protocols (HTTPS and FTPS) to ensure that communications are encrypted,
thereby protecting sensitive data.
There was a problem hiding this comment.
This code reuses the same logic from the existing NPM implementation (artifactory/commands/npm/publish.go#L127-L140)), which also reads hdr.Name from tar headers for in-memory package.json filtering. The tar header name is never used to perform any file system operations - it's only used for in-memory filtering, and content is read via io.ReadAll(tarReader).
If this needs to be addressed, it should be done in a separate PR for both NPM and PNPM together.
naveenku-jfrog
left a comment
There was a problem hiding this comment.
Lots of static and lint erros, and frogbot errors are present. These should be fixed before review.
@naveenku-jfrog static and lint errors appear because, as I mentioned in the PR description, it requires:
Regarding the Forgbot issues, the PNPM implementation reuses the same logic/code from the existing NPM implementation. These patterns already exist in NPM and were not flagged before. If any changes are warranted, they will be addressed in a separate PR for both NPM and PNPM together. WDYT? |


Related to feature request: RTECO-366
Requires:
Required for:
Related to:
Findings
All identified patterns match the existing npm implementation and are intentionally kept consistent:
.npmrcpermissions 0755pnpmcommand.go:325npmcommand.go:309pnpmcommand.go:195npmcommand.go:194pnpmcommand.go:271npmcommand.go:268These can be addressed in a future PR that updates both NPM and PNPM together if needed.